Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix outputFields logic conflicted between aggr & dynamic schema #585

Merged

Conversation

congqixia
Copy link
Contributor

Fix #584
/kind bug
This PR let output result set only check extra fields
If Milvus server returns more column than output fields specified, just return them

@sre-ci-robot sre-ci-robot added the kind/bug Something isn't working label Sep 19, 2023
@sre-ci-robot sre-ci-robot requested review from sunby and yah01 September 19, 2023 11:04
@sre-ci-robot sre-ci-robot added approved review approved size/M Denotes a PR that changes 30-99 lines. labels Sep 19, 2023
@codecov
Copy link

codecov bot commented Sep 19, 2023

Codecov Report

Merging #585 (da40f54) into master (849300d) will not change coverage.
Report is 3 commits behind head on master.
The diff coverage is 73.33%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #585   +/-   ##
=======================================
  Coverage   82.89%   82.89%           
=======================================
  Files          40       40           
  Lines        3647     3647           
=======================================
  Hits         3023     3023           
- Misses        511      512    +1     
+ Partials      113      112    -1     
Files Changed Coverage Δ
client/data.go 69.00% <73.33%> (-0.23%) ⬇️

... and 1 file with indirect coverage changes

@congqixia congqixia force-pushed the fix_aggr_conflict_dynamic_schema branch 2 times, most recently from 0102b30 to 0e617e2 Compare September 21, 2023 05:58
This PR let output result set only check extra fields
If Milvus server returns more column than output fields specified, just return them

Signed-off-by: Congqi Xia <[email protected]>
@congqixia congqixia force-pushed the fix_aggr_conflict_dynamic_schema branch from 0e617e2 to da40f54 Compare September 21, 2023 06:06
@mergify mergify bot added the ci-passed auto merge needed label label Sep 21, 2023
Copy link
Contributor

@jiaoew1991 jiaoew1991 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@sre-ci-robot sre-ci-robot added the lgtm look good to me label Sep 21, 2023
@sre-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: congqixia, jiaoew1991

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sre-ci-robot sre-ci-robot merged commit 2ba985a into milvus-io:master Sep 21, 2023
6 checks passed
@congqixia congqixia deleted the fix_aggr_conflict_dynamic_schema branch September 21, 2023 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved review approved ci-passed auto merge needed label kind/bug Something isn't working lgtm look good to me size/M Denotes a PR that changes 30-99 lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: query with count(*) returns error
3 participants